Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dropdown styles in Amsterdam #4301

Merged
merged 8 commits into from
Nov 24, 2020
Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Nov 23, 2020

This also includes some updates to the components of the default theme but shouldn't be any visual changes.

Changed EuiSuperSelect from using EuiPopover with custom styles to EuiInputPopover

This allowed me to remove a lot of placement logic that was already present in EuiInputPopover.

EuiComboBox now uses the same classes as EuiPopover's panel

Instead of duplicating styles and the increase in maintenance, I've copied the euiPopover__panel classes onto the EuiPanel within EuiComboBox to help maintain consistency of styles. I've added a comment in the code indicating that in a future re-factor of this component, it should just straight up use the EuiPopover component.

Also fixed the scrollbar and extra height issues.

image

Removed animation from EuiInputPopover (or whenever it is "attached")

This animation was just too much when it would end up connected to the input. So I only added the transition if the attached class doesn't exist.

Before
Screen Recording 2020-11-23 at 05 04 52 PM

After
Screen Recording 2020-11-23 at 05 05 25 PM

Fixed the date picker's dropdown and the datepicker range's input radius

This one was overlooked during the form updates. There's still more work to do on the datepicker itself, but this just fixes the immediate.

image

Also fixed #4218 by just completely removing the breakpoint that would hide the time select on mobile.

The picker just barely fit on a 320px wide screen anyway. We'll circle back to better mobile support when we re-write this component.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • [ ] Props have proper autodocs
  • [ ] Added documentation
  • [ ] Checked Code Sandbox works for the any docs examples
  • Added or updated jest tests
  • [ ] Checked for breaking changes and labeled appropriately
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4301/

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes make sense and code LGTM!

This highlighted for me more discrepancies with how we handle some interactions, so I created #4302. I don't see any reason to block this PR, though.

Copy link
Contributor

@miukimiu miukimiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Safari, Chrome, Firefox, and Edge. LGTM! 🎉

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4301/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiDatePicker] Support inline + showTimeSelectOnly on mobile
4 participants